Derive PCX plane count from the image, not the line width - #9798
Derive PCX plane count from the image, not the line width#9798chuenchen309 wants to merge 1 commit into
Conversation
When decoding a multi-plane (e.g. RGB) PCX scanline, the decoder recovered the per-plane stride by guessing the band count as state->bytes / xsize. state->bytes is planes * stride, so when the padded stride differs from the width (an odd-width line padded to an even stride) that quotient is wrong: for a width-3 RGB line it gives 4 bands / stride 3, the plane-compaction step is skipped, and the R/G/B planes stay interleaved. The pixels decode to the wrong colours with no error raised. Use the known band count (im->bands) to split the line instead. Added an RGB round-trip regression at a width that triggers the padding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
For the record, the idea was originally introduced in 5ecec7d, and the reason there are even-padded strides is because the PCX specification states that 'BytesPerLine' 'MUST be an EVEN number.' An alternate fix would be --- a/src/libImaging/PcxDecode.c
+++ b/src/libImaging/PcxDecode.c
@@ -69,7 +69,7 @@ ImagingPcxDecode(Imaging im, ImagingCodecState state, UINT8 *buf, Py_ssize_t byt
stride = state->bytes / state->bits;
} else {
xsize = state->xsize;
- bands = state->bytes / state->xsize;
+ bands = state->bytes / (state->xsize + (state->xsize % 2));
if (bands != 0) {
stride = state->bytes / bands;
}However, I think your fix is simpler. |
Testing, I find that the only failing scenario that your fix addresses is when the width is exactly 3. 3 pixels in width, plus 1 padding byte, makes a stride of 4. For every width other than 3, the calculation results in a number between 3 and 4, and so is rounded down to 3, the correct number of bands. So for 511, |
2 participants
Changes proposed in this pull request:
PcxDecode.crecovered the per-plane stride by guessing the band count asstate->bytes / state->xsize. Sincestate->bytesisplanes * stride, that quotient is wrong whenever theeven-padded stride differs from the width. For a width-3 RGB line it yields 4 bands / stride 3, the plane-compaction step is skipped, and the R/G/B planes stay interleaved — the image decodes to the wrong colours with no error raised. Derive the split from the known band count (
im->bands) instead.test_odduses a 511×511 RGB image, whose width makes the guess land on the right value by luck, so it never caught this.Scope note: at width 1 an RGB PCX still raises
OSErroron read — that is a separate problem on the encoder side (it emits only two of the three colour planes), with a different root cause, so I've left it out of this change.This PR was authored by an AI coding agent (Claude Code) running on this account: the AI found the bug (verified against the real C codec, not a simulation), ran the repro, wrote the test, and wrote this description. The human account holder reviews every change and is accountable for it. The verification is real and re-runnable from the diff. If this isn't the kind of contribution you want, say so and I'll close it.